fix(plugin-clickhouse): show the result grid whenever the server returns rows - #1896
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 886b7dedef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var isTruncated = false | ||
| for index in 2..<lines.count { | ||
| let line = lines[index] | ||
| if line.isEmpty { continue } |
There was a problem hiding this comment.
Preserve empty single-column TSV rows
When ClickHouse returns a one-column row whose value is the empty string, TabSeparatedWithNamesAndTypes represents that data row as an empty line, e.g. s\nString\n\n. This check skips every empty line, so queries such as SELECT '' AS s (including the comment-prefixed SELECTs this patch is meant to recover) are shown as zero rows even though the server returned a row. Only the final line separator should be discarded; an empty data line can be a real row.
Useful? React with 👍 / 👎.
Fixes #1886.
Root cause
The ClickHouse driver guessed whether a query returns data by taking the first token of the SQL, split on a literal space character.
SELECTon its own line yields the tokenSELECT\n, and a leading--or/* */comment yields--, so both misclassified a real SELECT. The misclassified path discarded the HTTP response body without reading it and returned empty columns, which the app renders as "Query executed successfully" with no grid. The same guess also gated the streaming export path, so exports of comment-prefixed queries wrote empty files.This is the bug class the ClickHouse JDBC maintainers fixed in clickhouse-java#2784: client-side statement classification must never make server-returned rows disappear.
Fix
Stop classifying the query text. Every request now pins the output format out of band and the response decides what to show:
default_format=TabSeparatedWithNamesAndTypesandwait_end_of_query=1go on the URL, never appended to the SQL. AppendingFORMAT Xto the SQL text was also independently broken: a trailing--comment swallowed it silently and a user-written FORMAT clause produced a double-FORMAT syntax error. Verified against a live ClickHouse 26.7 server.X-ClickHouse-Summaryheader'swritten_rows, so INSERT stops reporting 0 rows affected.X-ClickHouse-Formatresponse header): the raw output is shown in a single column instead of being dropped.http_write_exception_in_output_format=0is sent only to servers on 23.8 or later, gated throughClickHouseCapabilities, because older servers reject unknown settings.default_format=JSONEachRowinstead of string concatenation, closing the same hole there.The response classification and TSV parsing moved into
TableProPluginKitasClickHouseResponseClassifierso the shipped code is unit-testable.ClickHouseConnectionTestsnow tests that shipped code instead of a private copy of the unescape function.ABI
scripts/check-pluginkit-abi.sh mainreports an additive-only diff: one new public type, nothing removed or changed. No PluginKit version bump. ClickHouse is a bundled plugin, so this ships with the next app release.Testing
ClickHouseResponseClassifierTests(new),ClickHouseCapabilitiesTests(new), andClickHouseConnectionTests;xcodebuild testpasses.swiftlint lint --strictclean on changed files.Follow-up filed separately: #1895 (statement scanner turns a trailing comment after the last semicolon into a phantom statement).